Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #4247] Make all file inclusion lists configurable #5882

Merged
merged 1 commit into from
May 13, 2018

Conversation

jonas054
Copy link
Collaborator

Before this change, we were hard-coding file extensions, file names, and ruby interpreters, and then adding configured includes from AllCops/Include.

Now we add the parameter AllCops/RubyInterpreters and add **/*.rb to AllCops/Include.

By removing the hard-coded lists of things to inspect, this feature makes it possible to introduce RuboCop inspection slowly in a legacy project. Files and directories can be added for inspection incrementally. It's an alternative or complement to the existing workflow of adding cops one at a time using the --auto-gen-config feature.

Closes #5847, which was my first attempt at a fix for #4247.

Before this change, we were hard-coding file extensions, file
names, and ruby interpreters, and then adding configured
includes from AllCops/Include.

Now we add the parameter AllCops/RubyInterpreters and add
**/*.rb to AllCops/Include.

By removing the hard-coded lists of things to inspect, this
feature makes it possible to introduce RuboCop inspection
slowly in a legacy project. Files and directories can be added
for inspection incrementally. It's an alternative or complement
to the existing workflow of adding cops one at a time using the
--auto-gen-config feature.

Closes rubocop#5847, which was my first attempt at a fix for rubocop#4247.
@bbatsov bbatsov merged commit 1011c38 into rubocop:master May 13, 2018
@bbatsov
Copy link
Collaborator

bbatsov commented May 13, 2018

I love it! Great work, @jonas054! 👍

@stevenharman
Copy link

With this, how can I add a single entry to AllCops/Include without copy/pasting the entire list in the config/default.yml file?

@jonas054
Copy link
Collaborator Author

@stevenharman Currently you can't, and I think I have overlooked that possibility, thinking that inherit_mode could be used to specify that you want to merge AllCops/Include. But that doesn't work.

Could you open a new issue for this?

@stevenharman
Copy link

@jonas054 done: #5917

koic added a commit to koic/rubocop that referenced this pull request Aug 2, 2018
Fixes rubocop#6132.

### Summary

This PR fixes a false negative for `Naming/FileName`
when `Include` of `AllCops` is the default setting.

The important change in this PR is below.

```diff
diff --git a/lib/rubocop/cop/naming/file_name.rb
b/lib/rubocop/cop/naming/file_name.rb
index a025a73b7..be3f1c5aa 100644
--- a/lib/rubocop/cop/naming/file_name.rb
+++ b/lib/rubocop/cop/naming/file_name.rb
@@ -32,7 +32,7 @@ module RuboCop

         def investigate(processed_source)
           file_path = processed_source.file_path
-          return if config.file_to_include?(file_path)
+          return if config.file_to_exclude?(file_path)
```

The problem is that the target to be excluded was
`Include` instead of `Exclude`.

Also this PR adds the `RuboCop::Config#allowed_camel_case_file?`
method to judge ignoring `Gemfile`, `Rakefile`, etc
described in` Include`.

### Other Information

This false negative was noticed by adding `**/*.rb` to
config/default.yml at rubocop#5882.

https://github.com/rubocop-hq/rubocop/pull/5882/files#diff-e93280b3b31a6438c533a5f3232340d8R18
bbatsov pushed a commit that referenced this pull request Aug 3, 2018
Fixes #6132.

### Summary

This PR fixes a false negative for `Naming/FileName`
when `Include` of `AllCops` is the default setting.

The important change in this PR is below.

```diff
diff --git a/lib/rubocop/cop/naming/file_name.rb
b/lib/rubocop/cop/naming/file_name.rb
index a025a73b7..be3f1c5aa 100644
--- a/lib/rubocop/cop/naming/file_name.rb
+++ b/lib/rubocop/cop/naming/file_name.rb
@@ -32,7 +32,7 @@ module RuboCop

         def investigate(processed_source)
           file_path = processed_source.file_path
-          return if config.file_to_include?(file_path)
+          return if config.file_to_exclude?(file_path)
```

The problem is that the target to be excluded was
`Include` instead of `Exclude`.

Also this PR adds the `RuboCop::Config#allowed_camel_case_file?`
method to judge ignoring `Gemfile`, `Rakefile`, etc
described in` Include`.

### Other Information

This false negative was noticed by adding `**/*.rb` to
config/default.yml at #5882.

https://github.com/rubocop-hq/rubocop/pull/5882/files#diff-e93280b3b31a6438c533a5f3232340d8R18
jedcn added a commit to jedcn/reveal-ck that referenced this pull request Nov 22, 2018
It used to be that many more were included, but then this changed:

rubocop/rubocop#5882

And so now they need to be listed out explicitly.
gbp added a commit to mysociety/alaveteli that referenced this pull request Dec 3, 2018
This now includes the all standard file extensions included in the repo
which Rubocop can lint. We needed to be more explicit as the hardcode
list of extensions was removed with Rubocop.

See: rubocop/rubocop#5882
adamruzicka added a commit to adamruzicka/foreman-tasks that referenced this pull request Jun 13, 2019
The behavior of include changed in rubocop 0.56.0[1] and it started
overriding defaults provided by rubocop completely. This lead to only
rabl and rake files being checked by rubocop.

All of the file types which were manually included are being checked by
rubocop by default now and there's no need to keep them around.

[1] - rubocop/rubocop#5882
adamruzicka added a commit to adamruzicka/foreman-tasks that referenced this pull request Jun 13, 2019
The behavior of include changed in rubocop 0.56.0[1] and it started
overriding defaults provided by rubocop completely. This lead to only
rabl and rake files being checked by rubocop.

All of the file types which were manually included are being checked by
rubocop by default now and there's no need to manually include them.

[1] - rubocop/rubocop#5882
adamruzicka added a commit to adamruzicka/foreman-tasks that referenced this pull request Jul 17, 2019
The behavior of include changed in rubocop 0.56.0[1] and it started
overriding defaults provided by rubocop completely. This lead to only
rabl and rake files being checked by rubocop.

All of the file types which were manually included are being checked by
rubocop by default now and there's no need to manually include them.

[1] - rubocop/rubocop#5882
@eregon
Copy link

eregon commented Jan 26, 2020

Thank you for the PR.
However, I can't figure out how to use this, please see #4961 (comment)

Is it possible to have an include list of files to scan? RuboCop does not seem to respect it currently.

adamruzicka added a commit to adamruzicka/foreman-tasks that referenced this pull request Aug 7, 2020
The behavior of include changed in rubocop 0.56.0[1] and it started
overriding defaults provided by rubocop completely. This lead to only
rabl and rake files being checked by rubocop.

All of the file types which were manually included are being checked by
rubocop by default now and there's no need to manually include them.

[1] - rubocop/rubocop#5882
adamruzicka added a commit to theforeman/foreman-tasks that referenced this pull request Aug 7, 2020
* Fixes #27043 - Make rubocop work again

The behavior of include changed in rubocop 0.56.0[1] and it started
overriding defaults provided by rubocop completely. This lead to only
rabl and rake files being checked by rubocop.

All of the file types which were manually included are being checked by
rubocop by default now and there's no need to manually include them.

[1] - rubocop/rubocop#5882

* Fixes #27043 - Reconfigure rubocop rake tasks patterns

* Configure cops closer to core

Style/TrailingCommaInArrayLiteral
Style/TrailingCommaInHashLiteral
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants